Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always specify Cache-Control: no-cache in development #805

Merged
merged 4 commits into from
Jul 12, 2023

Conversation

matthew-white
Copy link
Member

Sometimes in local development, refreshing the page doesn't load the latest Frontend code: the same version of the page is shown as before. That can be especially confusing if the code change isn't visible in the UI, because it's harder to see that the page is outdated. Sometimes hard-refreshing the page doesn't even seem to load the latest code. In that case, I've had to restart npm run dev.

I've encountered this issue, and I know @ktuite and @alxndrsn have as well. Part of what's confusing is that it's not consistent (at least not for me). I can work for a long time without seeing this behavior.

Anecdotally, I feel like I've seen this issue much more since we've moved to Vue 3. Vue CLI isn't the preferred build tool for Vue 3, so I've been thinking that it's some issue with Vue CLI. In that case, the answer would probably be to move to Vite, which we want to do anyway (#671).

However, @ktuite was seeing this behavior even before Vue 3: see #520. Reading that issue, I do think that we need to change the Cache-Control header in development. I'm hoping that will just solve the issue, and that's the change that this PR makes. Even if that change doesn't fix things, we'll know more afterwards. I certainly don't think this change will hurt anything.

Another anecdote: I think I've seen this behavior most often for files in the main bundle. Whenever I modify src/router.js, it's fairly common for the change not to show up right away. However, most of the time, I'm working on components and files that components import, for example, composables and files in src/util/, and I feel like I see this issue much less often then. Most components are loaded asynchronously, which might have something to do with it. @alxndrsn reports having seen this issue while working on a component. However, the component he was working on, AccountLogin, is part of the main bundle, which seems consistent with the idea that the main bundle is a primary part of the issue. All that said, I'm pretty sure I've seen this issue outside the main bundle, just much less often (but more often now than with Vue 2).

There are other strategies we could try if this PR doesn't work. However, I think this is a good first step.

It's hard to know how to verify this PR given that I encounter the issue inconsistently. @alxndrsn, how consistently do you encounter the issue? Could you try out this PR and see if it helps? One thing I have verified is that Cache-Control is returned as no-cache for a range of Frontend files, not just index.html.

@matthew-white matthew-white requested a review from alxndrsn May 22, 2023 18:03
@matthew-white matthew-white linked an issue May 22, 2023 that may be closed by this pull request
@matthew-white
Copy link
Member Author

@alxndrsn, just wanted to see if you have time to take a quick look at this! Would be awesome if you could try it out and see if it helps with the issue you saw. I do think that it's an improvement, so part of me thinks that we should merge even if it doesn't end up solving your problem.

@alxndrsn
Copy link
Contributor

alxndrsn commented Jul 5, 2023

I haven't experimented much. I've been using the following for recent dev, but I'm not certain if it fixes issues, or if it's all relevant or necessary:

diff --git a/Procfile b/Procfile
index 5a38008a..3a73c6e9 100644
--- a/Procfile
+++ b/Procfile
@@ -1,2 +1,2 @@
-vue: vue-cli-service build --mode development --watch
+vue: vue-cli-service serve
 nginx: nginx -c "$PWD/main.nginx.conf" -p "$PWD"
diff --git a/main.nginx.conf b/main.nginx.conf
index 2026811f..ce5c1f7f 100644
--- a/main.nginx.conf
+++ b/main.nginx.conf
@@ -63,12 +63,14 @@ http {
 
     location /- {
       proxy_pass http://localhost:8005/-;
+      proxy_cache off;
       proxy_redirect off;
       proxy_set_header Host $host;
     }
 
     location ~ ^/v\d {
       proxy_pass http://localhost:8383;
+      proxy_cache off;
       proxy_redirect off;
 
       # buffer requests, but not responses, so streaming out works.
@@ -95,12 +97,9 @@ http {
     }
 
     location / {
-      root ./dist;
-
-      location /index.html {
-        include ./common-headers.nginx.conf;
-        add_header Cache-Control no-cache;
-      }
+      proxy_pass http://localhost:5000/;
+      proxy_cache off;
+      proxy_redirect off;
     }
   }
 }

@matthew-white
Copy link
Member Author

matthew-white commented Jul 11, 2023

Do you have any objection to the change in this PR? (Does the code look OK?) If it looks fine, I'd be interested to try merging it to see if it solves things.

@alxndrsn
Copy link
Contributor

I don't see any obvious issues with it. no-store might be a more intuitive choice than no-cache.

Is there any significance to the change in order of include vs add_header?

@matthew-white
Copy link
Member Author

Is there any significance to the change in order of include vs add_header?

I don't think so, I've reverted!

no-store might be a more intuitive choice than no-cache.

We use no-cache in production, and I think it keeps things simpler overall to reuse the same value here. Otherwise someone comparing the production vs. dev configs might wonder why they differ on this header.

@alxndrsn
Copy link
Contributor

We use no-cache in production, and I think it keeps things simpler overall to reuse the same value here. Otherwise someone comparing the production vs. dev configs might wonder why they differ on this header.

Should we be using no-store in prod then?

Copy link
Contributor

@alxndrsn alxndrsn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is confusing me, but I'm struggling to come up with a better alternative:

      # We return this header for fewer files in production. However, in
      # development, .js file names don't contain hashes.

@matthew-white
Copy link
Member Author

Should we be using no-store in prod then?

I agree that no-cache is a terrible name for the value given that it does in fact store content. But I think its behavior is what we want? Won't no-cache have better performance than no-store? If no-cache is used, then if the request for a file is revalidated, then I think the file won't need to be downloaded again. But the file is still required to be revalidated before reuse.

This comment is confusing me

Yeah, it could probably be clearer. 😅 How about:

We return this header for more files in development than we do in production. That's because in development, unlike production, many file names don't contain hashes.

@matthew-white matthew-white merged commit 0a3d8d2 into master Jul 12, 2023
@matthew-white matthew-white deleted the dev-cache-control branch July 12, 2023 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache-Control in development nginx.conf
2 participants